Codecov Report

Merging #678 (ad5b5d8) into main (5791e52) will decrease coverage by 2.03%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
- Coverage   21.24%   19.21%   -2.03%     
==========================================
  Files          12       13       +1     
  Lines         772     1046     +274     
==========================================
+ Hits          164      201      +37     
- Misses        583      801     +218     
- Partials       25       44      +19     

Note: this PR updates the registry v1 provisioner to mirror OLM V0 behavior, in which the namespace is set to the installNamespace if the resource does not specify a namespace.
/approve

This makes sense to me because its an implicit feature of the registry+v1 bundle format that objects without metadata.namespace defined will be created in the install namespace.

@awgreene, if a bundle object "A" hardcodes its namespace to "foo" and then someone creates a subscription resulting in the installation of that bundle in namespace "bar", where does "A" end up? "foo" or "bar"?

@joelanford my impression was that the namespace is always set to the installNamespace. Let me double check.

This is interesting @joelanford. Based on the ensure methods defined in step_ensurer.go, some objects end up in foo and some end up in bar:

• EnsureServiceAccount creates the object in bar. This applies for services, serviceAccounts, CSVs, subscriptions, secrets, services, roles, roleBindings
• EnsureUnstructuredObject creates the object in foo. This applies to objects not defined in the previous bullet but allowed in bundles.

NVM, @varshaprasad96 pointed out that the namespace for unstructured objects are set here, I thought that the ensure methods were responsible for setting the namespace as all ensure methods do other than EnsureUnstructureObject.

All namespaced resources would be created in the bar namespace @joelanford.

@varshaprasad96 for both of your questions, for better or worse, I think we should make registry+v1 bundles in OLMv1 behave as closely as possible to OLMv0. Reasons being:

1. There are years of expectations, assumptions, and muscle memory built up around the behavior of registry+v1 + OLMv0, to the point that registry+v1 is pretty much synonymous with OLMv0.
2. Any deviation from OLMv0 behavior with respect to registry+v1 bundles has the potential to cause a breaking change in migration scenarios.
3. Any new features or improvements we put in registry+v1 is one less reason for people to migrate to a different format. If we think registry+v1 is not what we want folks using long-term, it is perhaps better to avoid adding features to it that make it more appealing.

@joelanford does your answer change at all (maybe only from an implementation perspective) if/when we switch away from provisioners that are tied to bundle format? i.e. unpack-process-apply?

I was thinking about that last night. The idea that came to mind first was to expose something like renderRegistryV1 as a processor type, which would essentially call this same Convert function.

We probably have to stew on it more, but my instinct is that we should:

1. stick to our registry+v1 bundle validations.
2. make sure to support a simple unpack-process-apply setup that implements OLMv0 behavior to make migrations easy
3. allow BD clients to do more complex post-processing of registry+v1 bundles using more complex unpack-process-apply setups.